Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruff server: Fix multiple issues with Neovim and Helix #11497

Merged
merged 2 commits into from
May 22, 2024

Conversation

snowsignal
Copy link
Contributor

Summary

Fixes #11236.

This PR fixes several issues, most of which relate to non-VS Code editors (Helix and Neovim).

  1. Global-only initialization options are now correctly deserialized from Neovim and Helix
  2. Empty diagnostics are now published correctly for Neovim and Helix.
  3. A workspace folder is created at the current working directory if the initialization parameters send an empty list of workspace folders.
  4. The server now gracefully handles opening files outside of any known workspace, and will use global fallback settings taken from client editor settings and a user settings TOML, if it exists.

Test Plan

I've tested to confirm that each issue has been fixed.

  • Global-only initialization options are now correctly deserialized from Neovim and Helix + the server gracefully handles opening files outside of any known workspace
isolated_config.mov
  • Empty diagnostics are now published correctly for Neovim and Helix
empty_diagnostics.mov
  • A workspace folder is created at the current working directory if the initialization parameters send an empty list of workspace folders.
cwd_workspace.mov

@snowsignal snowsignal added bug Something isn't working configuration Related to settings and configuration server Related to the LSP server labels May 22, 2024
@snowsignal snowsignal added this to the Ruff Server: Beta milestone May 22, 2024
Copy link
Contributor

github-actions bot commented May 22, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@snowsignal snowsignal enabled auto-merge (squash) May 22, 2024 20:48
@snowsignal snowsignal merged commit 94abea4 into main May 22, 2024
16 checks passed
@snowsignal snowsignal deleted the jane/server/misc-fixes branch May 22, 2024 20:50
Comment on lines -133 to +134
settings: Option<ClientSettings>,
#[serde(flatten)]
settings: ClientSettings,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snowsignal when you're back, can you explain why is this being flattened? This is causing #11507 because now it's not under settings but rather in the global table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I understand why it was flattened - to account for the empty initialization options. I think we need to re-think about how to handle these cases.

dhruvmanila added a commit that referenced this pull request May 27, 2024
…ed (#11566)

## Summary

This PR fixes the bug to avoid flattening the global-only settings for
the new server.

This was added in #11497, possibly
to correctly de-serialize an empty value (`{}`). But, this lead to a bug
where the configuration under the `settings` key was not being read for
global-only variant.

By using #[serde(default)], we ensure that the settings field in the
`GlobalOnly` variant is optional and that an empty JSON object `{}` is
correctly deserialized into `GlobalOnly` with a default `ClientSettings`
instance.

fixes: #11507 

## Test Plan

Update the snapshot and existing test case. Also, verify the following
settings in Neovim:

1. Nothing

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
}
```

2. Empty dictionary

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = vim.empty_dict(),
}
```

3. Empty `settings`

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = {
    settings = vim.empty_dict(),
  },
}
```

4. With some configuration:

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = {
    settings = {
      configuration = '/tmp/ruff-repro/pyproject.toml',
    },
  },
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working configuration Related to settings and configuration server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ruff server fails to work in Neovim or Helix when outside of a workspace
3 participants